feat: converted src/APIs to TS #677
Conversation
|
@shaikahmadnawaz great! can you confirm your changes are working by running |
I got this 👇, check once, how can I pass all tests? Any .env?
|
|
@shaikahmadnawaz there's no need for an env variable - what OS are you running on? also to confirm, you have MongoDB locally on your machine and Java? |
|
@shaikahmadnawaz looks like the pipeline is failing due to a new TS change made in this PR - https://github.com/nkowaokwu/igbo_api/actions/runs/5340617462/jobs/9680629966?pr=677 |
The error message indicates that there is a type mismatch in my code. Specifically, the stems property of the Word interface is defined as an array of strings or objects with an optional id property.
|
|
|
One suggestion from my side, check whether this both APIs are working well before merging it to master branch. |
|
@shaikahmadnawaz the build step in GitHub is failing as expected since the type checking is failing - you'll need to make the appropriate change to allow TS to pass during the build step - the source of truth to ensure that this change is correct is our GitHub testing pipeline on this PR |
Okay, I will work on it. |
| }: HandleWordFlagsParams) => { | ||
| const updatedWords = compact( | ||
| words.map((word) => { | ||
| let updatedWord = assign({}, word); |
There was a problem hiding this comment.
Is there a reason why the Loadash is used instead of the native Object.assign method?
There was a problem hiding this comment.
nope, just a decision that was made in the past!
| const updatedData = assign(data); | ||
| interface SetCachedWordsParams { | ||
| key: string; | ||
| data: any; // Update the type of `data` accordingly |
There was a problem hiding this comment.
any takes away the benefit of using typescript, avoid if possible
|
@shaikahmadnawaz are you still working on this PR? |
I don't know why it is not working. |
|
@shaikahmadnawaz when you see a failing CI test you can click on the details button to see the step where the CI failed:
Here's the direct link to the error logs: https://github.com/nkowaokwu/igbo_api/actions/runs/5340819225/jo Looks like the RedisAPI is throwing the error saying that Error log
|
Got it and I removed |
RedisAPI is throwing the error saying that redis doesn't export RedisClient, so I removed import of this.
Describe your changes
This PR converts the
src/APIsdirectory to TypeScript by updating the file extensions from.jsto.ts. This change helps improve the maintainability and type safety of the codebase. No logical changes have been made to the files; only the file extensions and necessary type updates have been applied.Issue ticket number and link
Closed #673
Motivation and Context
The motivation behind this change is to enhance the codebase by leveraging TypeScript's static typing and improved developer tooling. By converting the src/APIs files to TypeScript, we can catch potential type errors during development and improve the overall robustness of the code.
How Has This Been Tested?
No specific tests have been performed for this PR since it primarily involves file extension changes. However, the codebase should be tested after merging this PR to ensure that the application continues to function correctly.
Screenshots (if appropriate):
N/A (No visual changes have been made)
Please review and merge this PR.